-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Upgrade [email protected] and [email protected] #360
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
@kevinswiber Thoughts on what support for v0.10 and v0.12 should be. The If people still need it they can use the current version of Zetta. Interesting to check what version of BBB are shipped with now. |
@AdamMagaluk Yes, I think it's time to say good-bye to v0.10 and v0.12. Choosing where to shift our compatibility promise is another issue. How well does Node 8 work on a Beagleboard? What do the latest boards ship with by default? Those are questions that might be important to answer. |
A change to the keepalive mechanism will be a change to the Z2Z protocol. Could we do it in a way that doesn't break compatibility? Z2Z is in need of a big overhaul. What we know today about HTTP/2 is so much more than what we knew then about SPDY/3. I'd make a few major changes, for sure. So if we need to break compatibility, how far do we go? |
Yeah we "can" do it without breaking changings will just need to make some modifications to the spdy library, no options to monkey patch it either right now. Agreed but for the scope of "just get zetta up-to-date current node" I think that probably deserves a separate discussion. I reached out to our friend Jason and Beaglebone to ask about the Node version. |
Update: Pings working again using a forked version of Found another issue while working on fixing the integration tests. For some reason when running two zetta servers within the same Node.js process the peering fails with full.js var zetta = require('../');
var MemoryRegistries = require('zetta-memory-registry')(zetta);
var PeerRegistry = MemoryRegistries.PeerRegistry;
var DeviceRegistry = MemoryRegistries.DeviceRegistry;
cloud = zetta({ registry: new DeviceRegistry(), peerRegistry: new PeerRegistry() });
cloud.name('A');
cloud.listen(0, function(err) {
if (err) {
throw err;
}
var cloudUrl = 'ws://localhost:' + cloud.httpServer.server.address().port;
setTimeout(function() {
z = zetta({ registry: new DeviceRegistry(), peerRegistry: new PeerRegistry() })
.name('B')
.link(cloudUrl)
.listen(0);
}, 1000)
})
|
CLAs look good, thanks! |
More progress... but issue above was fixed. Now working through the test suite and fixing mainly minor issues. Next major one is around the multiplexed event stream not working in some cases.
|
…ient. Fix issue with stream deduping when finding intenral remote port in the spdy internals.
Test Report getting closer.
|
Updates to the WS cause it to return a 400 instead of a 404 when the base path does not match.
…ribe's cb to fire a second time. Wrap cb in a once pattern. PeerSocket guard against this.ws not being initialized when closing the connection
Our tests are scheduling async operations and calling done. We need to clean this up but with the old version of mocha it was exiting the test suite after all tests were done. This enforces the old behavior.
Closing to create a new PR with cleaner history. |
Will hopefully address #336 and #333 also will help to modernize the codebase as it will support all the latest versions of node see: #337.
This PR has the basics working though there are a number of broken tests. Peering works. Websockets and data between peers all working tested with the trusty zetta browser.
The biggest roadblock right now is the
spdy
lib no longer supports a way of intercepting or sending spdy pings. Few options:Tested against with (simple example not unit/integration tests):
Breaks with v0.12 and v0.10 because of the ws module use of const.